Skip to content

Changes for passkey implementation#116

Open
rmad17 wants to merge 9 commits into
mainfrom
SDK-8780-passkey-feature
Open

Changes for passkey implementation#116
rmad17 wants to merge 9 commits into
mainfrom
SDK-8780-passkey-feature

Conversation

@rmad17

@rmad17 rmad17 commented Jun 1, 2026

Copy link
Copy Markdown

Changes

Passkey Authentication (ServerClient)

  • Signup challenge — initiates passkey registration for new users
  • Login challenge — initiates passkey authentication for existing users
  • Token exchange — completes passkey ceremony via WebAuthn grant type

MyAccount Credential Management (MyAccountClient)

  • List available factors for enrollment
  • Enroll authentication method (passkey, email, phone, TOTP, etc.)
  • Verify enrollment (WebAuthn response or OTP)
  • List / get / update / delete authentication methods
  • Optional DPoP token binding (RFC 9449, EC P-256) on all operations, with transparent Bearer fallback

References

@rmad17 rmad17 self-assigned this Jun 2, 2026
@rmad17 rmad17 marked this pull request as ready for review June 2, 2026 05:38
@rmad17 rmad17 requested a review from a team as a code owner June 2, 2026 05:38
client_extension_results: Optional[dict] = Field(None, alias="clientExtensionResults")


class VerifyAuthenticationMethodRequest(BaseModel):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these patterns necessary, or generated boilerplate?

A few things on the new models look possibly over-engineered — can you confirm each is intentional?

  • __repr__ redaction ([REDACTED]) is added to the new passkey models, but existing secret-bearing models in this same file (TokenSet, MfaVerifyResponse — both hold access_token / refresh_token / id_token) have no such redaction. Either we adopt redaction project-wide or drop it here for consistency; having it only on the new models is inconsistent.
  • @model_validator (_check_at_least_one_method) here — is this a real requirement or speculative validation?
  • model_config = ConfigDict(populate_by_name=True) — what's the reason this is needed on these models? If nothing populates by field name (vs alias), it can be removed.
  • EnrollAuthenticationMethodRequest.type is a free-form str while this file already uses Literal[...] for closed sets (AuthenticatorType, OobChannel, etc.). type (and preferred_authentication_method, which the spec restricts to sms/voice) could be Literal so bad values fail at construction instead of as a remote API error.
  • Minor: the enroll/verify docstrings say create:me:authentication_methods (underscore); the spec scope uses a hyphen — create:me:authentication-methods.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. __repr__ redaction - I have not made any changes to pre existing patterns to avoid accidentaly breaking code in a large PR. Though I feel the redaction should be there in all cases. It does not cost us anything rather its a better approach in terms of security.
  2. @model_validator (_check_at_least_one_method) This was added for better clarity on the error rather than a generic 400 for better DX. However, this breaks push notifications - Removed it.
  3. model_config = ConfigDict(populate_by_name=True) - Copy paste mistake. Since it does not break anything this did not get caught in tests.
  4. EnrollAuthenticationMethodRequest.type - Added literals.
  5. Updated the underscore to hyphens .


def auth_flow(self, request: httpx.Request):
proof = self._make_proof(request.method, str(request.url))
request.headers["Authorization"] = f"DPoP {self._token}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two DPoP gaps:

  1. DPoPAuth has no DPoP-Nonce handling. Auth0/Okta commonly responds 401 + a DPoP-Nonce header and expects the proof retried with that nonce, so the first call against a nonce-requiring resource server will fail. Can we track nonce-retry even if it's deferred past GA?
  2. Separately, signin_with_passkey does a plain token POST with no DPoP proof, and PasskeyTokenResponse.token_type defaults to Bearer, but the Test Plan says /oauth/token (webauthn) returns Bearer or DPoP based on tenant setup. If the tenant issues a DPoP-bound token, the SDK ends up holding a token bound to a key it never used. Can we confirm the GA scope for DPoP-bound passkey tokens?

@@ -0,0 +1,585 @@
import time

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests for methods that live in server_client.py should be added to test_server_client.py as a new section that mirrors the existing test patterns in that file (same fixtures, mocker.patch("httpx.AsyncClient.post", ...) / auth=ANY style). This file uses a different mocking idiom (patching _get_http_client with a hand-built async context manager) than the rest of the suite. let's keep it consistent.

Also note: the connect-account methods in my_account_client.py were reshaped alongside the formatting pass - please confirm no behavior changed there.

@@ -0,0 +1,830 @@
from unittest.mock import AsyncMock, MagicMock

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not introduce feature-scoped test files (test_passkey_my_account.py, test_passkey_server_client.py) at this point. We can keep using the existing test_my_account_client.py and test_server_client.py until we have a concrete reason to split files. Please fold these tests into the existing files.

# PASSKEY AUTHENTICATION
# ============================================================================

GRANT_TYPE_PASSKEY = "urn:okta:params:oauth:grant-type:webauthn"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please pull these up to a module/class-level constant rather than constructing them in the middle of the method body, consistent with how the rest of the client references endpoints.

# ============================================================================

GRANT_TYPE_PASSKEY = "urn:okta:params:oauth:grant-type:webauthn"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three methods implement the passkey login/signup ceremony: passkey_signup_challenge / passkey_login_challenge (step 1: get auth_session + the WebAuthn challenge), then the app runs navigator.credentials.create/get, then signin_with_passkey (step 2 :exchange the signed credential for tokens via the webauthn grant). Two questions:

  1. Can you confirm this is the intended end-to-end flow and that all three methods are exercised by it?
  2. signin_with_passkey returns a PasskeyTokenResponse but, unlike complete_interactive_login, never persists to the state store or creates a session. Is that intentional (caller owns the session), or should passkey sign-in integrate with the SDK's existing session/state handling like every other login path here?

For an RWA SDK whose value-add is session/state management, returning bare tokens reads as an inconsistency.

Comment on lines +2533 to +2561
if email is not None:
user_profile["email"] = email
if name is not None:
user_profile["name"] = name
if username is not None:
user_profile["username"] = username
if phone_number is not None:
user_profile["phone_number"] = phone_number
if given_name is not None:
user_profile["given_name"] = given_name
if family_name is not None:
user_profile["family_name"] = family_name
if nickname is not None:
user_profile["nickname"] = nickname
if picture is not None:
user_profile["picture"] = picture
if user_metadata is not None:
user_profile["user_metadata"] = user_metadata

body: dict[str, Any] = {"client_id": self._client_id}
if self._client_secret:
body["client_secret"] = self._client_secret
if user_profile:
body["user_profile"] = user_profile
if connection:
body["realm"] = connection
if organization:
body["organization"] = organization

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These if <field> is not None: user_profile[...] = <field> assignments are manually replicating what Pydantic gives for free. If we model user_profile as a typed sub-model on a request object (e.g. PasskeySignupChallengeOptions), this whole block reduces to:

if options.user_profile:                                                                                                                                                               
      body["user_profile"] = options.user_profile.model_dump(exclude_none=True) 

exclude_none=True preserves the current "omit unset keys" behavior, and each field gets real typing instead of an untyped Optional[str] kwarg.

We can use ConfigDict(extra="allow") to the sub-model also which will allow new profile fields pass through without touching this method.

if organization:
body["organization"] = organization

url = f"https://{domain}/passkey/register"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved here

"passkey_challenge_error",
f"Passkey signup challenge failed with status {response.status_code}",
)
raise ApiError(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a dedicated PasskeyError(Auth0Error) class (mirroring CustomTokenExchangeError) instead of ApiError with a passkey_challenge_error string code, so callers can catch the type directly.

except Exception as e:
if isinstance(e, (ApiError, MissingRequiredArgumentError, ValidationError)):
raise
raise ApiError("passkey_challenge_error", "Passkey signup challenge failed", e)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we catch an unexpected error here (like a network timeout) and re-raise it as an ApiError, the original error gets hidden, the caller only sees "Passkey signup challenge failed" and can't tell it was actually a connection problem. Adding from e (raise ApiError(...) from e) keeps the original error linked in the traceback so it's still debuggable.

Same applies to the equivalent line in passkey_login_challenge and signin_with_passkey.

Comment thread src/auth0_server_python/auth_server/server_client.py Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants